fix(sdk): prevent list_directory path escapes - #1188
Conversation
|
This looks like a real, well-scoped security fix.
One thing worth double-checking on port: using No forbidden paths touched (sdk/, common/ only). Good, minimal diff for the actual behavior change, with tests matching the existing bun:test conventions. This is worth porting as-is. |
codebuff-team
added
bot:triaged
|
Rechecked the |
Approving the path and reading it are two separate lookups, so the directory the boundary check approved is not necessarily the one readdir opens: swap a path component for a symlink pointing outside the project in between and the listing comes back from wherever the swap pointed, having passed the check. Node has no readdir on a descriptor, so the read cannot be pinned to the inode that was approved. Pin identity around it instead - the directory that was approved, the one that was read, and the one still at that path afterwards must all be the same inode, and the path must still resolve inside the project. An attacker who restores the path before the recheck still wins, so this narrows the window rather than closing it; the comment says so rather than implying the check is airtight. Two tests cover it: a directory that changes inode across the read, and a path that starts resolving outside the project. Both fail without this change - the second one returns a listing from outside the project.
2 participants
Summary
realpath.Fixes #463
Validation
Prior validation before the history rewrite:
bun run --cwd common typecheckbun run --cwd sdk typecheckbun run --cwd sdk test— 518 passed, 0 failedbun test common/src/__tests__/project-file-tree.test.ts— 12 passed, 0 failedgit diff --check